Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow count in data sources #8635

Merged
merged 4 commits into from
Sep 3, 2016
Merged

Allow count in data sources #8635

merged 4 commits into from
Sep 3, 2016

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Sep 2, 2016

Count worked in data sources, but the resource validation would fail due to the extra "count" key in the raw config. Make sure the HCL loader removes count.

Fixes #8527

@mitchellh
Copy link
Contributor

mitchellh commented Sep 2, 2016

Looks really great!

A couple things:

  • Could we add a context_apply_test as well for this to make sure that the multiple query works properly?
  • Can we add a test to verify count.index works properly within a data source? (e.x. foo = "${count.index}")

@jbardin jbardin force-pushed the jbardin/GH-8527 branch 2 times, most recently from 36c91b4 to 387ae8a Compare September 2, 2016 20:45
@jbardin
Copy link
Member Author

jbardin commented Sep 2, 2016

@mitchellh Can you give me an example of what you mean by that first part? I couldn't come up with a good test for apply that didn't just repeat the plan. It's probably the same issue I had with assigning "${count.index}" -- I couldn't figure out how to expose them correctly in the tests (the data sources show up differently in the state than regular resources).

@jen20
Copy link
Contributor

jen20 commented Sep 2, 2016

@mitchellh I'm not so sure there is a distinct apply test here because of the refresh at plan to make the values known?

@mitchellh
Copy link
Contributor

You're probably right with refresh and I can add a count.index test later.

Data sources should be able to support counts like a resource. We need
to remove "count" when we load the config because the key doesn't exist
in the schema, and the resource won't validate.
@jen20
Copy link
Contributor

jen20 commented Sep 3, 2016

Also fixes #7919 (and probably some others).

This adds a unit test to the test provider that verifies count.index
behaves correctly. Although not ideal this is hard to implement as a
context test without changing around the (non helper/schema)
implementation of the x_data_source.
@jen20
Copy link
Contributor

jen20 commented Sep 3, 2016

I've added a test of count.index, but to avoid circular dependencies or large modifications to the null providers used in context tests it is implemented as a unit test in the builtin/providers/test package. It's possible that there is a better way to test this in future however!

@ghost
Copy link

ghost commented Apr 22, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

count is not implemented in data sources
4 participants